fix - Persist cancelled tool call status when chat is cancelled#240
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an inconsistency between the live chat UI and persisted conversation state when a chat is cancelled while an agent tool call is still running, ensuring restored conversations don’t incorrectly show tool calls as still “running”.
Changes:
- Update the chat cancel flow to persist cached conversations after first converting any persisted
runningtool-call statuses tocancelled. - Add persistence-layer logic to walk cached conversations and update tool-call statuses before saving.
- Add a unit test to validate that only
runningtool calls are changed tocancelled.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java | Switches cancel flow persistence call to a new “mark running tool calls cancelled + persist” API. |
| com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java | Adds a new persistence method that updates cached tool-call statuses before persisting. |
| com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java | Adds coverage verifying running tool calls are converted to cancelled while completed calls are untouched. |
Comments suppressed due to low confidence (2)
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java:296
markRunningToolCallsCancelledAndPersistreturns early (and skips persistence) when no tool call status is updated. This changes behavior from the previouspersistCachedConversationcall in the cancel flow: cancelling a chat with no currently-runningtool calls would no longer persist the cached conversation state to disk. Consider always persisting when the conversation exists (only conditionally mutating tool-call statuses), and update the Javadoc/logic accordingly.
public CompletableFuture<Void> markRunningToolCallsCancelledAndPersist(String conversationId) {
if (StringUtils.isBlank(conversationId)) {
return CompletableFuture.completedFuture(null);
}
ConversationData conversationData;
lock.writeLock().lock();
try {
conversationData = conversationCache.get(conversationId);
if (conversationData == null) {
return CompletableFuture.completedFuture(null);
}
if (!markRunningToolCallsCancelled(conversationData)) {
return CompletableFuture.completedFuture(null);
}
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java:305
- This method acquires the write lock and walks the conversation data synchronously before scheduling the async persist. Since
ChatView.onCancel()can call this on the UI thread, it can block UI responsiveness if other background persistence/progress tasks are holding the same lock. Consider moving the cache lookup + status update + persist into theCompletableFuture.runAsyncblock so the UI thread never waits on the lock.
ConversationData conversationData;
lock.writeLock().lock();
try {
conversationData = conversationCache.get(conversationId);
if (conversationData == null) {
return CompletableFuture.completedFuture(null);
}
if (!markRunningToolCallsCancelled(conversationData)) {
return CompletableFuture.completedFuture(null);
}
} finally {
lock.writeLock().unlock();
}
return CompletableFuture.runAsync(() -> {
lock.writeLock().lock();
try {
persistAndCacheConversation(conversationData);
} catch (IOException e) {
jdneo
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #239.
When a chat response is cancelled while an agent tool call is still running, the live UI shows the tool as cancelled, but the cached conversation can still persist that tool call with
runningstatus. Restoring the conversation later reads that persisted status and shows the tool call as running/spinning again.This change marks cached
runningtool calls ascancelledbefore persisting the conversation during the chat cancel flow.Changes
runningtool calls are updated while completed tool calls remain unchanged.